Skip to content

Conversation

@ValwareIRC
Copy link
Member

This module displays a specific message in the MOTD based on a defined date.

This module displays a specific message in the MOTD based on a defined date.
Copilot AI review requested due to automatic review settings November 11, 2025 09:58
Copilot finished reviewing on behalf of ValwareIRC November 11, 2025 10:00
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new UnrealIRCd module that displays date-specific messages in the MOTD (Message of the Day). The module supports both specific dates (YYYY-MM-DD format) and recurring weekday-based messages.

Key Changes:

  • Implements date-based MOTD functionality with support for specific dates and weekday names
  • Adds configuration parsing for datemotd blocks with name, file, and date fields
  • Includes date validation and matching logic for both formats

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

struct datemotd_item *next = p->next;
safe_free(p->name);
safe_free(p->file);
safe_free(p->date); // NEW
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment "NEW" is a development artifact that should be removed before finalizing the code. This type of comment is useful during development but serves no purpose in the final codebase.

Suggested change
safe_free(p->date); // NEW
safe_free(p->date);

Copilot uses AI. Check for mistakes.
Comment on lines +147 to +148
sendnumeric(client, RPL_MOTD, line);
}
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file reading logic doesn't handle lines that don't end with a newline character (e.g., the last line of a file). If the last line doesn't have a trailing newline, line[len-1] won't be '\n', and the line will be sent with its content intact but won't be processed through the newline removal logic. This could lead to inconsistent behavior. Consider also sending lines that don't end with newlines.

Suggested change
sendnumeric(client, RPL_MOTD, line);
}
}
sendnumeric(client, RPL_MOTD, line);

Copilot uses AI. Check for mistakes.
Comment on lines +138 to +140
FILE *fp = fopen(file, "r");
if (!fp)
return;
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no error logging when the file cannot be opened. Users won't know why their configured datemotd isn't being displayed. Consider logging a warning or error message to help with troubleshooting.

Copilot uses AI. Check for mistakes.
Comment on lines +72 to +90
const char *days_short[] = { "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat" };
const char *days_long[] = { "Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday" };

char lower_input[16];
int i;
for (i = 0; i < sizeof(lower_input)-1 && datestr[i]; i++)
lower_input[i] = tolower((unsigned char)datestr[i]);
lower_input[i] = '\0';

for (i = 0; i < 7; i++) {
char tmp[16];
snprintf(tmp, sizeof(tmp), "%s", days_short[i]);
for (int j = 0; tmp[j]; j++) tmp[j] = tolower((unsigned char)tmp[j]);
if (!strcmp(lower_input, tmp)) return 1;

snprintf(tmp, sizeof(tmp), "%s", days_long[i]);
for (int j = 0; tmp[j]; j++) tmp[j] = tolower((unsigned char)tmp[j]);
if (!strcmp(lower_input, tmp)) return 1;
}
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The weekday name arrays and lowercase conversion logic are duplicated in three places: is_valid_date_string (lines 72-90), is_today (lines 112-128), and the arrays appear again in is_today. Consider extracting this into a helper function or using constants to avoid code duplication and reduce maintenance burden.

Copilot uses AI. Check for mistakes.
return 0;

time_t now = time(NULL);
struct tm *tm_now = localtime(&now);
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

localtime() returns a pointer to static internal storage that may be overwritten by subsequent calls. While this is likely safe in this specific context, it's not thread-safe. Consider using localtime_r() for thread safety if the UnrealIRCd framework supports multi-threaded operations.

Copilot uses AI. Check for mistakes.
}


// NEW: hook to send MOTD only on the matching date
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment "NEW: hook to send MOTD only on the matching date" is a development artifact that should be removed. This type of comment is useful during development but serves no purpose in the final codebase.

Suggested change
// NEW: hook to send MOTD only on the matching date

Copilot uses AI. Check for mistakes.
Comment on lines +82 to +84
char tmp[16];
snprintf(tmp, sizeof(tmp), "%s", days_short[i]);
for (int j = 0; tmp[j]; j++) tmp[j] = tolower((unsigned char)tmp[j]);
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable j is declared in the for loop initialization, which requires C99 or later. While this is likely supported, ensure consistency with the rest of the codebase's C standard usage. The same pattern appears on line 88.

Copilot uses AI. Check for mistakes.
Comment on lines +62 to +67
if (isdigit(datestr[0])) {
int y, m, d;
if (sscanf(datestr, "%d-%d-%d", &y, &m, &d) != 3)
return 0;
if (y < 1970 || m < 1 || m > 12 || d < 1 || d > 31)
return 0;
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The date validation doesn't account for valid days per month (e.g., February 31st would pass validation). Consider adding proper date validation that checks the actual number of days in each month, including leap years.

Copilot uses AI. Check for mistakes.
const char *days_long[] = { "Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday" };

char lower_input[16];
int i;
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The buffer size check i < sizeof(lower_input)-1 should use strlen(datestr) or ensure the input length doesn't exceed the buffer. If datestr is longer than 15 characters, it will be silently truncated, which could lead to unexpected behavior for invalid inputs.

Suggested change
int i;
int i;
// Reject input longer than buffer size (15 chars)
if (strlen(datestr) > sizeof(lower_input) - 1)
return 0;

Copilot uses AI. Check for mistakes.
struct datemotd_item {
char *name;
char *file;
char *date; // NEW: date in format YYYY-MM-DD
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment "NEW: date in format YYYY-MM-DD" is incomplete and misleading. The date field actually supports both YYYY-MM-DD format and weekday names (e.g., "Monday", "Mon"). Update the comment to reflect all supported formats.

Suggested change
char *date; // NEW: date in format YYYY-MM-DD
char *date; // Date in format YYYY-MM-DD or weekday name (e.g., "Monday", "Mon")

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant